Skip to content

Conversation

@inner-daemons
Copy link
Collaborator

@inner-daemons inner-daemons commented Oct 19, 2025

Connections
Closes #8341
Required for #8376

Description
The CreateGraphicsPipelineState function works great for basic render pipelines, but for more advanced features such as mesh shaders (#8110) or multiview/view instancing (#8206), you must use a streaming descriptor. The streaming descriptor corresponds more or less to a vulkan pipeline descriptor with the state described in a pNext chain, allowing you to use all sorts of extensions.

This PR switches all pipeline creation to use this function. Mesh shaders already used it but there was no reason to use different code paths for mesh shader pipelines vs standard pipelines when 99% of the descriptor is the same.

One thing to note: if it is ever possible in some esoteric system to use the DX12 backend on a 32-bit system, this code might not function properly. It probably still would, but it might not. Not that this is a setup that any real person has of course.

Testing
Existing testing (pretty much all tests use render pipelines)

Squash or Rebase?

Squash

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We likely can't use stream descriptors unconditionally unless we can prove that support for it goes back far enough

@cwfitzgerald
Copy link
Member

Alright, so it was the Creators Update (1703) that introduced it, we'd need to hear from mozilla if that's too new.

@inner-daemons
Copy link
Collaborator Author

I can probably switch this so that the pipeline descriptor gets converted into a graphics pipeline descriptor on platforms where this wouldn't work. That probably will be the best for compatibility regardless of what Mozilla wants.

@cwfitzgerald cwfitzgerald self-assigned this Oct 22, 2025
@cwfitzgerald
Copy link
Member

Discussed at today's wgpu matainers meeting. Resolution: lets use this conditionally. mapping from a single struct works fine.

Discussion
  • EG: RE: platform coverage: Please don’t make downstreams use this unconditionally. Firefox needs to support Windows 10, for now.
  • CF: Can we just do this unconditionally? No, because Firefox needs to support Windows 10.
  • EG: Is there a benefit (besides consolidated code) to using this init path unconditionally, even if somebody isn’t using the features blocked on its usage?
  • CF: I’m not concerned about code quality, or continuing to support Windows 10, or anything here, just wanted to drop code we don’t need if we can.

@cwfitzgerald
Copy link
Member

This is waiting on getting refactored into its own optional file.

@inner-daemons inner-daemons force-pushed the dx12-pipeline-stream-desc branch from ae3040d to ac012d7 Compare November 5, 2025 18:45
@inner-daemons
Copy link
Collaborator Author

It's now in its own file, and falls back to standard pipeline descriptors on windows versions that don't support stream descs. Should be good to go?

@inner-daemons inner-daemons mentioned this pull request Nov 8, 2025
6 tasks
@inner-daemons
Copy link
Collaborator Author

Going to mention for the record that @Wumpf has accidentally reviewed this PR in his review on #8495, offering tons of great feedback.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, basically done.

@cwfitzgerald cwfitzgerald force-pushed the dx12-pipeline-stream-desc branch 5 times, most recently from bd51b65 to f742607 Compare November 14, 2025 20:40
@cwfitzgerald cwfitzgerald force-pushed the dx12-pipeline-stream-desc branch from f742607 to 7d496d7 Compare November 14, 2025 21:06
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored a bit as discussed, and this looks both provably sound and ready to land.

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) November 14, 2025 21:06
auto-merge was automatically disabled November 14, 2025 21:21

Head branch was pushed to by a user without write access

@cwfitzgerald
Copy link
Member

Oops, thanks

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) November 14, 2025 21:22
@cwfitzgerald cwfitzgerald merged commit 92fa99a into gfx-rs:trunk Nov 14, 2025
41 checks passed
@inner-daemons inner-daemons deleted the dx12-pipeline-stream-desc branch November 15, 2025 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch DX12 pipeline creation to always use a stream descriptor

2 participants